fix(sentryapps): Cache Sentry App installation lookups to prevent blocking RPCs#113493
fix(sentryapps): Cache Sentry App installation lookups to prevent blocking RPCs#113493sentry[bot] wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
Scope: Backend
shayna-ch
added
the
Trigger: getsentry tests
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| payload={ | ||
| "sentry_app_id": self.sentry_app_id, | ||
| "organization_id": self.organization_id, | ||
| "proxy_user_id": self.sentry_app.proxy_user_id, |
There was a problem hiding this comment.
SentryApp.DoesNotExist not handled in outboxes_for_delete when accessing proxy_user_id
The new code at line 170 accesses self.sentry_app.proxy_user_id without handling SentryApp.DoesNotExist. Since SentryApp uses ParanoidModel (soft-delete), the parent SentryApp may be soft-deleted before the installation's outboxes_for_delete is called. The same file already demonstrates this pattern at lines 146-149 where api_application_id catches SentryApp.DoesNotExist. This will cause deletion to fail with an unhandled exception when the SentryApp has been deleted first.
Verification
Read sentry_app_installation.py to verify: 1) SentryApp FK relationship at line 72, 2) Existing DoesNotExist handling pattern at lines 146-149 for similar access to self.sentry_app.application_id, 3) SentryApp is a ParanoidModel (soft-delete) confirmed in sentry_app.py line 91. The same exception risk applies to proxy_user_id access.
Identified by Warden sentry-backend-bugs · JDV-DGX
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
1 participant
This PR addresses a 'Blocking Operation' performance issue (SENTRY-5NC9) occurring on
/api/0/organizations/{organization_id_or_slug}/events-stats/when accessed by Sentry Apps (e.g., Grafana integration proxy users).Root Cause:
Each Sentry App request to a region endpoint was making two separate, synchronous cross-silo RPC HTTP calls to the control silo during the request lifecycle:
app_service.get_installation_org_id_by_token_id(inratelimits/utils.pyfor rate-limiting) - ~318ms blocking.app_service.find_installation_by_proxy_user(inauth/access.pyfor permission checks) - ~248ms blocking.These calls were necessary because Sentry App installation data resides in the control silo, and the region silo had no local cache for these specific lookups.
Solution:
This fix introduces Redis caching for the results of these two RPC calls in the region silo, significantly reducing latency for Sentry App requests.
Changes Made:
src/sentry/sentry_apps/services/app/service.py:get_installation_org_id_by_token_idas a cached wrapper around the RPC call, using a Django cache keysentry-app-install-token:{token_id}with a 300s TTL.get_installation_by_proxy_useras a cached wrapper around the RPC call, using a Django cache keysentry-app-install-proxy:{proxy_user_id}:{organization_id}with a 300s TTL._INSTALLATION_NOT_FOUND_SENTINELandclear_installation_by_proxy_user_cachehelper for cache management.src/sentry/ratelimits/utils.py:get_organization_id_from_tokento utilize the new cachedget_installation_org_id_by_token_idfunction.src/sentry/auth/access.py:_from_rpc_sentry_appto utilize the new cachedget_installation_by_proxy_userfunction.src/sentry/sentry_apps/models/sentry_app_installation.py:outboxes_for_deleteto includeproxy_user_idin the payload, enabling proper cache invalidation on deletion.handle_async_replicationandhandle_async_deletionto clear thesentry-app-install-proxycache key when aSentryAppInstallationis updated or deleted.Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Fixes SENTRY-5NC9